Netlist Writer: write post synthesis netlist that can be simulated #1957

lpawelcz · 2022-01-25T13:47:00Z

This PR is a work in progress. It relies on code from 3 different PRs:

That is why it's based on top of concatenated commits from all 3 PRs. This will cause those commits to appear on the list of commits in this PR and in the diff. I will update this PR when the rest of PRs will be merged.

Description

This PR modifies the VPR Netlist Writer in order to allow writing Verilog and SDF files that can be simulated. The changes include:

changing the access specifiers of some members of class NetlistWriterVisitor from private to protected
adding class MergedNetlistWriterVisitor that inherits from NetlistWriterVisitor and overrides some of its behaviour in order to write another verilog netlist that will have its multi-bit top module ports merged so that we have code like this:

module top (
    input \clk ,
    output [3:0] led);

instead of this:

module top (
    input \clk ,
    output \led[0] ,
    output \led[1] ,
    output \led[2] ,
    output \led[3] 
);

adding new option --gen_post_synthesis_merged_netlist that enables second verilog writer
changing the format of multi-bit ports in cells instancing
escaping characters [ and ] in Black Box cell instancing in SDF files

Related Issue

Issue was first described in #1946.

Motivation and Context

This change is required for performing simulation of post synthesis verilog files written by VPR without using any external scripts for adjusting VPR output. Simulations are part of Symbiflow testing procedure where verilog files with merged multi-bit ports of at least the top module are needed.

How Has This Been Tested?

New VPR build was used to generate verilog output for Symbiflow designs that were then simulated to verify the correctness of verilogs

Types of changes

Bug fix (change which fixes an issue)
New feature (change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My change requires a change to the documentation
I have updated the documentation accordingly
I have added tests to cover my changes
All new and existing tests passed

Signed-off-by: Pawel Czarnecki <[email protected]>

Change access specifiers from private to protected for some members of NetlistWriterVisitor to make those visible in class that will derive from the visitor. Signed-off-by: Pawel Czarnecki <[email protected]>

…ntifiers Signed-off-by: Pawel Czarnecki <[email protected]>

Signed-off-by: Pawel Czarnecki <[email protected]>

Signed-off-by: Paweł Czarnecki <[email protected]>

acomodi

Looks good to me, I believe that @vaughnbetz had some comments regarding the naming of the --post_synthesis_netlist step which might need to be changed to something else, e.g. --post_implementation_netlist or similar, as it happens after the whole place and route stages.

…ged_netlist Signed-off-by: Paweł Czarnecki <[email protected]>

vaughnbetz

Looks good to me. Just suggest changing the name thoughout from post_synthesis to post_implementation; also have a few minor commenting suggestions and fixes.

vaughnbetz · 2022-04-08T16:14:10Z

doc/src/vpr/command_line_usage.rst

@@ -1254,6 +1254,14 @@ Analysis Options

    **Default:** ``off``

+.. option:: --gen_post_synthesis_merged_netlist { on | off }
+
+    This option is based on ``--gen_post_synthesis_netlist``.


typo: "a single verilog file"
I think we should change the name to --gen_post_implementation_netlist, as @acomodi suggests.

Fixed typo and renamed option

vaughnbetz · 2022-04-08T16:14:50Z

doc/src/vpr/command_line_usage.rst

+    This option is based on ``--gen_post_synthesis_netlist``.
+    The difference is that ``--gen_post_synthesis_merged_netlist`` generates only single verilog file with merged top module multi-bit ports of the implemented circuit.
+    The name of the file is ``<basename>_merged_post_synthesis.v``
+


Should probably rename the output file to _post_implementation.v

vaughnbetz · 2022-04-08T16:15:04Z

vpr/src/base/SetupVPR.cpp

@@ -623,6 +623,7 @@ static void SetupAnalysisOpts(const t_options& Options, t_analysis_opts& analysi
    }

    analysis_opts.gen_post_synthesis_netlist = Options.Generate_Post_Synthesis_Netlist;
+    analysis_opts.gen_post_synthesis_merged_netlist = Options.Generate_Post_Synthesis_Merged_Netlist;



name -> post_implementation

vaughnbetz · 2022-04-08T16:32:48Z

vpr/src/base/netlist_writer.cpp

@@ -2099,6 +2120,187 @@ class NetlistWriterVisitor : public NetlistVisitor {
    struct t_analysis_opts opts_;
 };

+/**
+ * @brief A class which writes post-synthesis merged netlists (Verilog)


post synthesis --> post implementation

vaughnbetz · 2022-04-08T16:35:36Z

vpr/src/base/netlist_writer.cpp

@@ -2124,6 +2326,23 @@ void netlist_writer(const std::string basename, std::shared_ptr<const AnalysisDe
    nl_walker.walk();
 }

+///@brief Main routing for this file. See netlist_writer.h for details.
+void merged_netlist_writer(const std::string basename, std::shared_ptr<const AnalysisDelayCalculator> delay_calc, struct t_analysis_opts opts) {


I think "Main routing" should be "Main routine"

vaughnbetz · 2022-04-08T16:36:02Z

vpr/src/base/read_options.h

@@ -208,6 +208,7 @@ struct t_options {
    /* Analysis options */
    argparse::ArgValue<bool> full_stats;
    argparse::ArgValue<bool> Generate_Post_Synthesis_Netlist;
+    argparse::ArgValue<bool> Generate_Post_Synthesis_Merged_Netlist;
    argparse::ArgValue<int> timing_report_npaths;


Same comment: post_synthesis -> post_implementation

lpawelcz · 2022-04-11T08:32:36Z

@acomodi, @vaughnbetz - thanks for the reviews! I've renamed all occurrences of post_synthesis_merged_netlist (and similar) to post_implementation_merged_netlist and fixed all typos that were pointed out.

Signed-off-by: Paweł Czarnecki <[email protected]>

vaughnbetz · 2022-04-11T18:39:16Z

vpr/src/base/netlist_writer.cpp

+  protected:
+    /**
+     * @brief Returns the name of a wire connecting a primitive and global net.
+     *


What does global net mean in this context? Just a net that is routed? (I'm guessing that).

Or an actual global (often not routed, on a special network) net?

Would be good to clarify.

vaughnbetz · 2022-04-11T18:40:58Z

Looks good to me.
A couple of clean up things that would be good to do after merge.

What is a global net in the netlister_writer.cpp comment (see my note above)?
Would be good to update the gen_post_synthesis_netlist option, and any related options, to gen_post_implementation_netlist (new name) too.

github-actions bot added the VPR VPR FPGA Placement & Routing Tool label Jan 25, 2022

lpawelcz force-pushed the pcza/merge-verilog-ports branch from 97ce5f6 to acbde85 Compare March 28, 2022 09:18

lpawelcz added 9 commits March 29, 2022 13:15

vpr: base: netlist writer: refactor verilog writer

bcd49bb

Signed-off-by: Pawel Czarnecki <[email protected]>

vpr: base: netlist writer: change access specifiers

1965f2d

Change access specifiers from private to protected for some members of NetlistWriterVisitor to make those visible in class that will derive from the visitor. Signed-off-by: Pawel Czarnecki <[email protected]>

vpr: base: netlist writer: assignment: add print without escaping ide…

9766400

…ntifiers Signed-off-by: Pawel Czarnecki <[email protected]>

vpr: base: introduce netlist writer for merged verilog ports

d9ec991

Signed-off-by: Pawel Czarnecki <[email protected]>

vpr: base: add option for dumping merged netlist

ff949b9

Signed-off-by: Pawel Czarnecki <[email protected]>

vpr: base: merged netlist writer: add opts for unconnected ports

7d912c3

Signed-off-by: Pawel Czarnecki <[email protected]>

vpr: base: netlist writer: split multi-bit cell instance ports

fe45554

Signed-off-by: Pawel Czarnecki <[email protected]>

vpr: base: netlist writer: escape '[' and ']' for black boxes in SDF

8135ebb

Signed-off-by: Pawel Czarnecki <[email protected]>

doc: vpr: cmd: document --gen_post_synthesis_merged_netlist

8b09d1b

Signed-off-by: Pawel Czarnecki <[email protected]>

lpawelcz force-pushed the pcza/merge-verilog-ports branch 2 times, most recently from c2ee265 to 1c6150c Compare April 6, 2022 12:57

lpawelcz marked this pull request as ready for review April 6, 2022 12:57

lpawelcz changed the title ~~[WIP]: Netlist Writer: write post synthesis netlist that can be simulated~~ Netlist Writer: write post synthesis netlist that can be simulated Apr 6, 2022

vpr: base: netlist writer: change formatting

7952b53

Signed-off-by: Paweł Czarnecki <[email protected]>

lpawelcz force-pushed the pcza/merge-verilog-ports branch from 1c6150c to 7952b53 Compare April 6, 2022 16:44

lpawelcz requested review from acomodi and vaughnbetz April 7, 2022 08:06

acomodi reviewed Apr 8, 2022

View reviewed changes

vpr: rename gen_post_synthesis_netlist to gen_post_implementation_mer…

6e9a897

…ged_netlist Signed-off-by: Paweł Czarnecki <[email protected]>

vaughnbetz reviewed Apr 8, 2022

View reviewed changes

lpawelcz force-pushed the pcza/merge-verilog-ports branch from b003083 to c17f3dd Compare April 11, 2022 08:22

vpr: fix typos

12830cd

Signed-off-by: Paweł Czarnecki <[email protected]>

lpawelcz force-pushed the pcza/merge-verilog-ports branch from c17f3dd to 12830cd Compare April 11, 2022 10:22

vaughnbetz reviewed Apr 11, 2022

View reviewed changes

vaughnbetz merged commit 829c06d into verilog-to-routing:master Apr 11, 2022

lpawelcz mentioned this pull request Jun 30, 2022

VPR: add second netlist writer for merged multi-bits ports lnis-uofu/OpenFPGA#700

Merged

8 tasks

lpawelcz mentioned this pull request Jul 15, 2022

Revert "vpr: base: netlist writer: escape '[' and ']' for black boxes… #2096

Merged

7 tasks

Netlist Writer: write post synthesis netlist that can be simulated #1957

Netlist Writer: write post synthesis netlist that can be simulated #1957

Uh oh!

Conversation

lpawelcz commented Jan 25, 2022

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

acomodi left a comment

Choose a reason for hiding this comment

Uh oh!

vaughnbetz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lpawelcz commented Apr 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vaughnbetz commented Apr 11, 2022

Uh oh!

Uh oh!

lpawelcz commented Apr 11, 2022 •

edited

Loading